refactor(metric provider): use explore style time ranges #2603
Conversation
filipgutica
force-pushed
the
fix/update-metric-provider-timeframe
branch
2 times, most recently
from
7ce4d8b to
f214e76
Compare
| datasource?: QueryDatasource | ||
| maxTimeframe?: TimeframeKeys | ||
| overrideTimeframe?: Timeframe | ||
| tz?: string |
There was a problem hiding this comment.
Removed tz as a prop as it can just be provided with the time range?
adorack
left a comment
adorack
left a comment
There was a problem hiding this comment.
(Haven't looked at ui-apps yet.)
I think as long as some of the queryTime code remains, the tests that exercise it should remain as well. I think that means it's feasible to remove a lot of the tests, but there are some absolute time tests in there that are probably still relevant?
| datasource?: QueryDatasource | ||
| maxTimeframe?: TimeframeKeys | ||
| overrideTimeframe?: Timeframe | ||
| tz?: string |
| expect(new TimeseriesQueryTime(getTimePeriod(TimeframeKeys.PREVIOUS_MONTH)).startDate().getHours()).toEqual(0) | ||
| }) | ||
|
|
||
| it('custom timeframe daily', () => { |
There was a problem hiding this comment.
This test, at least, should probably stay here...
| }) | ||
| }) | ||
|
|
||
| describe('non-timeseries queries with custom timeframes', () => { |
There was a problem hiding this comment.
This test should probably remain.
| expect(deltaQuery.granularitySeconds()).toBe(unaryQuery.granularitySeconds()) | ||
| }) | ||
|
|
||
| it('determines a correct trend query across multiple transitions', () => { |
There was a problem hiding this comment.
This test should probably remain.
| } | ||
| return relativePeriod | ||
| const overrideTimeRange: Ref<TimeRangeV4> = computed(() => { |
There was a problem hiding this comment.
This no longer needs to be a computed -- you can just reference the variable directly.
filipgutica
force-pushed
the
fix/update-metric-provider-timeframe
branch
from
f214e76 to
ce4ebb0
Compare
BREAKING CHANGE: metric provider props updated to accept explore style timerange
filipgutica
force-pushed
the
fix/update-metric-provider-timeframe
branch
from
3228f5f to
3c83a89
Compare
| import { ref } from 'vue' | ||
| import type { ExploreResultV4 } from '@kong-ui-public/analytics-utilities' | ||
| import type { ExploreResultV4, | ||
| Timeframe } from '@kong-ui-public/analytics-utilities' |
There was a problem hiding this comment.
This seems like it shouldn't need the Timeframe type any more.
Summary
BREAKING CHANGE: metric provider props updated to accept explore style time range